From: Yehuda Katz Date: Tue, 13 May 2014 00:33:13 +0000 (-0700) Subject: Wrote more integration tests for cargo compile X-Git-Tag: archive/raspbian/0.35.0-2+rpi1~3^2^2^2^2^2^2^2~1061 X-Git-Url: https://dgit.raspbian.org/%22http://www.example.com/cgi/success//%22http:/www.example.com/cgi/success/?a=commitdiff_plain;h=21322f07b4ca2fea12dbeacd8122f292f768ab99;p=cargo.git Wrote more integration tests for cargo compile At the moment, the rustc exec for the root project inherits the stdout and stderr FDs (so that warnings and errors flow through), but output from dependencies is only emitted if the compilation fails to avoid warning noise from dependencies. --- diff --git a/src/bin/cargo-compile.rs b/src/bin/cargo-compile.rs index 39bc267d1..269ec346b 100644 --- a/src/bin/cargo-compile.rs +++ b/src/bin/cargo-compile.rs @@ -32,5 +32,5 @@ fn execute(options: Options) -> CLIResult> { CLIError::new("Could not find Cargo.toml in this directory or any parent directory", Some(err.to_str()), 102))) }; - compile(root.as_str().unwrap().as_slice()).map(|v| Some(v)).to_cli(101) + compile(root.as_str().unwrap().as_slice()).map(|_| None).to_cli(101) } diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 513106601..a208517f6 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -105,9 +105,18 @@ pub struct PackageSet { impl PackageSet { pub fn new(packages: &[Package]) -> PackageSet { + assert!(packages.len() > 0, "PackageSet must be created with at least one package") PackageSet { packages: Vec::from_slice(packages) } } + pub fn len(&self) -> uint { + self.packages.len() + } + + pub fn pop(&mut self) -> Package { + self.packages.pop().unwrap() + } + /** * Get a package by name out of the set */ diff --git a/src/cargo/mod.rs b/src/cargo/mod.rs index b0ad6c23e..aaaa7f60e 100644 --- a/src/cargo/mod.rs +++ b/src/cargo/mod.rs @@ -60,7 +60,7 @@ pub fn execute_main_without_stdin<'a, T: RepresentsFlags, V: Encodable, io::IoError>>(result: CLIResult>) { @@ -76,8 +76,11 @@ pub fn process_executed<'a, T: Encodable, io::IoError>>(result } pub fn handle_error(err: CLIError) { - let _ = write!(&mut std::io::stderr(), "{}", err.msg); - std::os::set_exit_status(err.exit_code as int); + let CLIError { msg, exit_code, .. } = err; + let _ = write!(&mut std::io::stderr(), "{}", msg); + //detail.map(|d| write!(&mut std::io::stderr(), ":\n{}", d)); + + std::os::set_exit_status(exit_code as int); } fn flags_from_args() -> CLIResult { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index e1522caaa..919a6b90b 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -44,7 +44,7 @@ pub fn compile(manifest_path: &str) -> CargoResult<()> { try!(source.download(resolved.as_slice()).wrap("unable to download packages")); - let packages = try!(source.get(resolved.as_slice()).wrap("unable ot get packages from source")); + let packages = try!(source.get(resolved.as_slice()).wrap("unable to get packages from source")); let package_set = PackageSet::new(packages.as_slice()); diff --git a/src/cargo/ops/cargo_rustc.rs b/src/cargo/ops/cargo_rustc.rs index e93129f92..ab9e7acab 100644 --- a/src/cargo/ops/cargo_rustc.rs +++ b/src/cargo/ops/cargo_rustc.rs @@ -1,27 +1,34 @@ use std::os::args; use std::io; use std::path::Path; +use std::str; use core; use util; -use util::{other_error,CargoResult,CargoError}; +use util::{other_error,human_error,CargoResult,CargoError,ProcessBuilder}; +use util::result::ProcessError; type Args = Vec<~str>; pub fn compile(pkgs: &core::PackageSet) -> CargoResult<()> { - let sorted = match pkgs.sort() { + let mut sorted = match pkgs.sort() { Some(pkgs) => pkgs, None => return Err(other_error("circular dependency detected")) }; + let root = sorted.pop(); + for pkg in sorted.iter() { println!("Compiling {}", pkg); - try!(compile_pkg(pkg, pkgs)); + try!(compile_pkg(pkg, pkgs, |rustc| rustc.exec_with_output())); } + println!("Compiling {}", root); + try!(compile_pkg(&root, pkgs, |rustc| rustc.exec())); + Ok(()) } -fn compile_pkg(pkg: &core::Package, pkgs: &core::PackageSet) -> CargoResult<()> { +fn compile_pkg(pkg: &core::Package, pkgs: &core::PackageSet, exec: |&ProcessBuilder| -> CargoResult) -> CargoResult<()> { // Build up the destination // let src = pkg.get_root().join(Path::new(pkg.get_source().path.as_slice())); let target_dir = pkg.get_absolute_target_dir(); @@ -31,7 +38,7 @@ fn compile_pkg(pkg: &core::Package, pkgs: &core::PackageSet) -> CargoResult<()> // compile for target in pkg.get_targets().iter() { - try!(rustc(pkg.get_root(), target, &target_dir, pkgs.get_packages())) + try!(rustc(pkg.get_root(), target, &target_dir, pkgs.get_packages(), |rustc| exec(rustc))) } Ok(()) @@ -41,19 +48,24 @@ fn mk_target(target: &Path) -> io::IoResult<()> { io::fs::mkdir_recursive(target, io::UserRWX) } -fn rustc(root: &Path, target: &core::Target, dest: &Path, deps: &[core::Package]) -> CargoResult<()> { +fn rustc(root: &Path, target: &core::Target, dest: &Path, deps: &[core::Package], exec: |&ProcessBuilder| -> CargoResult) -> CargoResult<()> { + let rustc = prepare_rustc(root, target, dest, deps); + + try!(exec(&rustc) + .map_err(|err| rustc_to_cargo_err(rustc.get_args(), root, err))); + + Ok(()) +} + +fn prepare_rustc(root: &Path, target: &core::Target, dest: &Path, deps: &[core::Package]) -> ProcessBuilder { let mut args = Vec::new(); build_base_args(&mut args, target, dest); build_deps_args(&mut args, deps); - try!(util::process("rustc") + util::process("rustc") .cwd(root.clone()) .args(args.as_slice()) - .exec() - .map_err(|err| rustc_to_cargo_err(&args, root, err))); - - Ok(()) } fn build_base_args(into: &mut Args, target: &core::Target, dest: &Path) { @@ -73,7 +85,17 @@ fn build_deps_args(dst: &mut Args, deps: &[core::Package]) { } } -fn rustc_to_cargo_err(args: &Vec<~str>, cwd: &Path, err: io::IoError) -> CargoError { - other_error("failed to exec rustc") - .with_detail(format!("args={}; root={}; cause={}", args.connect(" "), cwd.display(), err.to_str())) +fn rustc_to_cargo_err(args: &[~str], cwd: &Path, err: CargoError) -> CargoError { + let msg = { + let output = match err { + CargoError { kind: ProcessError(_, ref output), .. } => output, + _ => fail!("Bug! exec() returned an error other than a ProcessError") + }; + + let mut msg = StrBuf::from_str(format!("failed to execute: `rustc {}`", args.connect(" "))); + output.as_ref().map(|o| msg.push_str(format!("; Error:\n{}", str::from_utf8_lossy(o.error.as_slice())))); + msg + }; + + human_error(msg.to_owned(), format!("root={}", cwd.display()), err) } diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 22c84efc3..fd80bfb6d 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -1,5 +1,5 @@ pub use self::process_builder::{process,ProcessBuilder}; -pub use self::result::{CargoError,CargoResult,Wrap,Require,ToCLI,other_error,human_error,toml_error}; +pub use self::result::{CargoError,CargoResult,Wrap,Require,ToCLI,other_error,human_error,toml_error,io_error,process_error}; pub mod graph; pub mod process_builder; diff --git a/src/cargo/util/process_builder.rs b/src/cargo/util/process_builder.rs index 1740c9a4a..e8aea0e0e 100644 --- a/src/cargo/util/process_builder.rs +++ b/src/cargo/util/process_builder.rs @@ -2,8 +2,8 @@ use std::fmt; use std::fmt::{Show,Formatter}; use std::os; use std::path::Path; -use std::io; use std::io::process::{Process,ProcessConfig,ProcessOutput,InheritFd}; +use util::{CargoResult,io_error,process_error}; use collections::HashMap; #[deriving(Clone,Eq)] @@ -36,6 +36,10 @@ impl ProcessBuilder { self } + pub fn get_args<'a>(&'a self) -> &'a [~str] { + self.args.as_slice() + } + pub fn extra_path(mut self, path: Path) -> ProcessBuilder { // For now, just convert to a string, but we should do something better self.path.push(format!("{}", path.display())); @@ -48,7 +52,7 @@ impl ProcessBuilder { } // TODO: should InheritFd be hardcoded? - pub fn exec(&self) -> io::IoResult<()> { + pub fn exec(&self) -> CargoResult<()> { let mut config = try!(self.build_config()); let env = self.build_env(); @@ -57,31 +61,34 @@ impl ProcessBuilder { config.stdout = InheritFd(1); config.stderr = InheritFd(2); - let mut process = try!(Process::configure(config)); + let mut process = try!(Process::configure(config).map_err(io_error)); let exit = process.wait(); if exit.success() { Ok(()) - } - else { - Err(io::IoError { - kind: io::OtherIoError, - desc: "process did not exit successfully", - detail: None - }) + } else { + let msg = format!("Could not execute process `{}`", self.debug_string()); + Err(process_error(msg, exit, None)) } } - pub fn exec_with_output(&self) -> io::IoResult { + pub fn exec_with_output(&self) -> CargoResult { let mut config = try!(self.build_config()); let env = self.build_env(); config.env = Some(env.as_slice()); - Process::configure(config).map(|mut ok| ok.wait_with_output()) + let output = try!(Process::configure(config).map(|mut ok| ok.wait_with_output()).map_err(io_error)); + + if output.status.success() { + Ok(output) + } else { + let msg = format!("Could not execute process `{}`", self.debug_string()); + Err(process_error(msg, output.status.clone(), Some(output))) + } } - fn build_config<'a>(&'a self) -> io::IoResult> { + fn build_config<'a>(&'a self) -> CargoResult> { let mut config = ProcessConfig::new(); config.program = self.program.as_slice(); @@ -91,6 +98,10 @@ impl ProcessBuilder { Ok(config) } + fn debug_string(&self) -> ~str { + format!("{} {}", self.program, self.args.connect(" ")) + } + fn build_env(&self) -> ~[(~str, ~str)] { let mut ret = Vec::new(); diff --git a/src/cargo/util/result.rs b/src/cargo/util/result.rs index 2645b0775..1692e1ab8 100644 --- a/src/cargo/util/result.rs +++ b/src/cargo/util/result.rs @@ -1,4 +1,8 @@ +use std::fmt; +use std::fmt::{Show,Formatter}; use std::io; +use std::io::IoError; +use std::io::process::{ProcessOutput,ProcessExit}; use core::errors::{CLIError,CLIResult}; use toml; @@ -18,6 +22,26 @@ pub fn other_error(desc: &'static str) -> CargoError { } } +pub fn io_error(err: IoError) -> CargoError { + let desc = err.desc; + + CargoError { + kind: IoError(err), + desc: StaticDescription(desc), + detail: None, + cause: None + } +} + +pub fn process_error(detail: ~str, exit: ProcessExit, output: Option) -> CargoError { + CargoError { + kind: ProcessError(exit, output), + desc: BoxedDescription(detail), + detail: None, + cause: None + } +} + pub fn human_error(desc: ~str, detail: ~str, cause: CargoError) -> CargoError { CargoError { kind: HumanReadableError, @@ -38,7 +62,7 @@ pub fn toml_error(desc: &'static str, error: toml::Error) -> CargoError { #[deriving(Show,Clone)] pub struct CargoError { - kind: CargoErrorKind, + pub kind: CargoErrorKind, desc: CargoErrorDescription, detail: Option<~str>, cause: Option> @@ -85,15 +109,45 @@ impl CargoError { } } -#[deriving(Show,Clone)] pub enum CargoErrorKind { HumanReadableError, InternalError, + ProcessError(ProcessExit, Option), IoError(io::IoError), TomlError(toml::Error), OtherCargoError } +impl Show for CargoErrorKind { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + match self { + &ProcessError(ref exit, _) => write!(f.buf, "ProcessError({})", exit), + &HumanReadableError => write!(f.buf, "HumanReadableError"), + &InternalError => write!(f.buf, "InternalError"), + &IoError(ref err) => write!(f.buf, "IoError({})", err), + &TomlError(ref err) => write!(f.buf, "TomlError({})", err), + &OtherCargoError => write!(f.buf, "OtherCargoError") + } + } +} + +impl Clone for CargoErrorKind { + fn clone(&self) -> CargoErrorKind { + match self { + &ProcessError(ref exit, ref output) => { + ProcessError(exit.clone(), output.as_ref().map(|output| ProcessOutput { + status: output.status.clone(), output: output.output.clone(), error: output.error.clone() + })) + }, + &HumanReadableError => HumanReadableError, + &InternalError => InternalError, + &IoError(ref err) => IoError(err.clone()), + &TomlError(ref err) => TomlError(err.clone()), + &OtherCargoError => OtherCargoError + } + } +} + type CargoCliResult = Result; #[deriving(Show,Clone)] diff --git a/tests/support.rs b/tests/support.rs index 49dc81032..338564da5 100644 --- a/tests/support.rs +++ b/tests/support.rs @@ -8,7 +8,8 @@ use std::str; use std::vec::Vec; use std::fmt::Show; use ham = hamcrest; -use cargo::util::{process,ProcessBuilder}; +use cargo::util::{process,ProcessBuilder,CargoError}; +use cargo::util::result::ProcessError; static CARGO_INTEGRATION_TEST_DIR : &'static str = "cargo-integration-tests"; @@ -230,6 +231,7 @@ impl ham::Matcher for Execs { match res { Ok(out) => self.match_output(&out), + Err(CargoError { kind: ProcessError(_, ref out), .. }) => self.match_output(out.get_ref()), Err(_) => Err(format!("could not exec process {}", process)) } } diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index a909f802f..15b29c284 100644 --- a/tests/test_cargo_compile.rs +++ b/tests/test_cargo_compile.rs @@ -6,19 +6,23 @@ use cargo::util::process; fn setup() { } -test!(cargo_compile { - let p = project("foo") - .file("Cargo.toml", r#" - [project] +fn basic_bin_manifest(name: &str) -> ~str { + format!(r#" + [project] - name = "foo" - version = "0.5.0" - authors = ["wycats@example.com"] + name = "{}" + version = "0.5.0" + authors = ["wycats@example.com"] - [[bin]] + [[bin]] - name = "foo" - "#) + name = "{}" + "#, name, name) +} + +test!(cargo_compile { + let p = project("foo") + .file("Cargo.toml", basic_bin_manifest("foo")) .file("src/foo.rs", main_file(r#""i am foo""#, [])); assert_that(p.cargo_process("cargo-compile"), execs()); @@ -50,6 +54,84 @@ test!(cargo_compile_without_manifest { .with_stderr("Could not find Cargo.toml in this directory or any parent directory")); }) +test!(cargo_compile_with_invalid_code { + let p = project("foo") + .file("Cargo.toml", basic_bin_manifest("foo")) + .file("src/foo.rs", "invalid rust code!"); + + let target = p.root().join("target"); + + assert_that(p.cargo_process("cargo-compile"), + execs() + .with_status(101) + .with_stderr(format!("src/foo.rs:1:1: 1:8 error: expected item but found `invalid`\nsrc/foo.rs:1 invalid rust code!\n ^~~~~~~\nfailed to execute: `rustc src/foo.rs --crate-type bin --out-dir {} -L {}`", target.display(), target.display()))); +}) + +test!(cargo_compile_with_warnings_in_the_root_package { + let p = project("foo") + .file("Cargo.toml", basic_bin_manifest("foo")) + .file("src/foo.rs", "fn main() {} fn dead() {}"); + + assert_that(p.cargo_process("cargo-compile"), + execs() + .with_stderr("src/foo.rs:1:14: 1:26 warning: code is never used: `dead`, #[warn(dead_code)] on by default\nsrc/foo.rs:1 fn main() {} fn dead() {}\n ^~~~~~~~~~~~\n")); +}) + +test!(cargo_compile_with_warnings_in_a_dep_package { + let mut p = project("foo"); + let bar = p.root().join("bar"); + + p = p + .file(".cargo/config", format!(r#" + paths = ["{}"] + "#, bar.display())) + .file("Cargo.toml", r#" + [project] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + + [dependencies] + + bar = "0.5.0" + + [[bin]] + + name = "foo" + "#) + .file("src/foo.rs", main_file(r#""{}", bar::gimme()"#, ["bar"])) + .file("bar/Cargo.toml", r#" + [project] + + name = "bar" + version = "0.5.0" + authors = ["wycats@example.com"] + + [[lib]] + + name = "bar" + "#) + .file("bar/src/bar.rs", r#" + pub fn gimme() -> ~str { + "test passed".to_owned() + } + + fn dead() {} + "#); + + assert_that(p.cargo_process("cargo-compile"), + execs() + .with_stdout("Compiling bar v0.5.0\nCompiling foo v0.5.0\n") + .with_stderr("")); + + assert_that(&p.root().join("target/foo"), existing_file()); + + assert_that( + cargo::util::process("foo").extra_path(p.root().join("target")), + execs().with_stdout("test passed\n")); +}) + test!(cargo_compile_with_nested_deps { let mut p = project("foo"); let bar = p.root().join("bar");